This is removed and now all parse operations use mimalloc.

A problem with this approach is that not all cores are guaranteed to have called EngineShard::InitThreadLocal and have a memory resource using the mimalloc heap initialized.

This is seen in the failing tests. If the num_shards flag is set to less than the number of cores, some cores will not have a thread local heap for parsing JSON.

There are places in code which just need to parse some string into json without relying on mimalloc heap (eg in cluster config). These places use the default memory resource. They need to use the same allocator which uses the mimalloc heap.

There are places in code which just need to parse some string into json without relying on mimalloc heap (eg in cluster config). These places use the default memory resource. They need to use the same allocator which uses the mimalloc heap.

The right thing to do here seems to be to defer all parsing of JSON objects until we are on the shard, in the tx callback. At present some parsing is done on coordinator thread to return errors early, and some on the shard where the key resides.

The other uses of JsonFromString such as cluster config module can use the default pmr resource. As they do not interact with compact obj. and related heap.

will be rebased after #6061, which will introduce two paths for parsing JSON values:

• parse on any shard using default PMR resource, not intended to use as compact object OR
• parse on transaction destination shard using mimalloc heap, intended to use as compact object in db

@abhijat I am not sure I get it. Why can not we use default resource in coordinator thread together with JSONFromString, and use ShardJsonFromString in shard threads together with thread local heap?

Ok, I understand now:

it is possible to work around this by using two allocator types, one for the default heap and one for mimalloc heap. but since the allocator type is part of the json object type, going down this path means that we now have two non interchangeable json object types

but where do they meet? i.e. do we pass a "coordinator" json objects to shard threads? in other words, even today I would expect that those objects would be "non-interchangeable", as we must allocate shard local objects only with shard local heap, so in a way having two disjoin c++ types would make things more explicit and clear.

but where do they meet? i.e. do we pass a "coordinator" json objects to shard threads? in other words, even today I would expect that those objects would be "non-interchangeable", as we must allocate shard local objects only with shard local heap,

There are a few places where we allocate on coordinator using the default memory resource and pass to the destination shard which uses mimalloc based allocation, and then mix those objects. I think it is done to facilitate early error reporting, but I have changed those call sites in #6061 deferring json parsing to the destination shard.

In fact I found this comment https://github.com/dragonflydb/dragonfly/blob/main/src/server/json_family.cc#L1905-L1907 which mentions this same problem.

so in a way having two disjoin c++ types would make things more explicit and clear.

Yes, I added a new type in that PR called ShortLivedJSON (name is a bit confusing but not sure what else to name it) - which uses the standard allocator, in fact it is an alias for the default json type of jsoncons. It is now used in most places where we do json parsing with std::pmr::get_default_resource()

After that parent PR is merged, the other JSONType which we use everywhere in the code can be changed to use stateless allocator, and the two types will be incompatible and never expected to work with each other.

Great!

I think this is in good condition now once tests pass. I would like to rename JsonType to something like PrimeValueJson or PvJson to make it clear that it is intended for use as prime value but that can be a separate PR as it will touch a lot of files.